Allow registering a custom Predicate for determining non-blocking threads
#3847
Conversation
…hreads Related issue: reactor#3833 Motivation: It is currently not possible to create a non-blocking threads without implementing the `reactor.core.scheduler.NonBlocking` interface. Some third-party libraries and frameworks don't directly depend on `reactor-core`, yet they want to mark the threads they manage as non-blocking. Modifications: - Added a new method `Schedulers.registerNonBlockingThreadPredicate()` so that a user can register their own `Predicate` that determines whether a given thread is non-blocking or not - Fixed an incorrectly implemented test that doesn't really test anything: - `SchedulersTest.isInNonBlockingThreadTrue()` Result: - Closes reactor#3833 - A user can now mark their own `Thread` classes as non-blocking without depending on `reactor-core` or implementing the `NonBlocking` marker interface.
|
@trustin Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
1 similar comment
|
@trustin Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
|
@trustin Thank you for signing the Contributor License Agreement! |
There was a problem hiding this comment.
Thank you for following up with the PR. I have just one request, but overall it looks neat and will be happy to merge once the reset functionality is added. Also, please notice the build outcome. I expect it to fail due to license headers not being updated. Please run ./gradlew spotlessApply locally.
| assertNonBlockingThread(CustomNonBlockingThread::new, true); | ||
| } finally { | ||
| // Restore the global predicate. | ||
| Schedulers.nonBlockingThreadPredicate = Schedulers.DEFAULT_NON_BLOCKING_THREAD_PREDICATE; |
There was a problem hiding this comment.
This is possible only because the package is the same. Please provide a reset functionality (back to the default thread -> false) so that end users in their packages can also validate the behaviour of their custom Predicate but also make sure they can validate the behaviour when it's not registered and the test state does not leak.
There was a problem hiding this comment.
Added Schedulers.resetNonBlockingThreadPredicate() and updated the test.
| future.complete(null); | ||
| } catch (Throwable cause) { | ||
| future.completeExceptionally(cause); | ||
| } |
There was a problem hiding this comment.
I like this pattern, thanks! In other places we set the check using some atomic type, utilize a latch and then combine the await with an assertion. This allows to use the assertions API directly, awesome.
|
Just noticed the branch is targeting main. As this is an additive change, I think we could ship this in 3.6.9. Or are you ok to delay this until 3.7.0? Adding functionality in patch brings a bit of an effort to the usages where the actual runtime library might have the API or might not so you need a way to check if you can register the |
- Added `Schedulers.resetNonBlockingThreadPredicate()` - Updated `SchedulersTest` to use and test `resetNonBlockingThreadPredicate()`
|
That's a good idea, @chemicL. Let me send another PR that targets 3.6 soon. 🙇 |
|
Thanks, closing this one then. |
Related issue: #3833
Motivation:
It is currently not possible to create a non-blocking threads without implementing the
reactor.core.scheduler.NonBlockinginterface. Some third-party libraries and frameworks don't directly depend onreactor-core, yet they want to mark the threads they manage as non-blocking.Modifications:
Schedulers.registerNonBlockingThreadPredicate()so that a user can register their ownPredicatethat determines whether a given thread is non-blocking or notSchedulers.resetNonBlockingThreadPredicate()so that a user can reset the registrations.SchedulersTest.isInNonBlockingThreadTrue()Result:
Threadclasses as non-blocking without depending onreactor-coreor implementing theNonBlockingmarker interface.